Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding CODEOWNERS #21581

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding CODEOWNERS #21581

wants to merge 1 commit into from

Conversation

hamzaremmal
Copy link
Member

[skip ci]

@lrytz
Copy link
Member

lrytz commented Sep 13, 2024

IIUC

  • owners / owner teams will be notified and assigned as reviewers on PRs that change matching files
  • we can set up a rule to require reviews from an owner to enable PR merging

What is the benefit over the status quo, it it solving a problem, or addressing some security concern?

@hamzaremmal
Copy link
Member Author

What is the benefit over the status quo, it it solving a problem, or addressing some security concern?

So each line in the file will contain a rule, and the rule will consist of a glob and the team that is allowed to approve changes to matching files. For now, I'm suggesting to require the admins approval on sensible files and the infrastructure team for changes in the .github/workflows folder. Over the status quo, anybody can approve any changes so the proposed solution is more secure.

@mbovel
Copy link
Member

mbovel commented Sep 13, 2024

I understand that some cleanup around the Scala organization was probably needed. However, my two cents is that we should be cautious about adding further restrictions just for the sake of it. The potential negative impact on usability and the time it takes to deal with these restrictions may outweigh the security benefits. From a community perspective, we should also make sure that these measures don’t lead to contributors feeling mistrusted or infantilized.

@SethTisue
Copy link
Member

SethTisue commented Nov 5, 2024

well, I haven't been feeling the lack of this

still, I would say that as it stands this seems mergeable, at least as a trial, since only a few files are listed and the ownership on the files listed is so clear

but I don't think there should be any expectation that we gradually expand this to cover lots of files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants